-
Couldn't load subscription status.
- Fork 0
FIX - 220 - Handle enums correctly #221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report
@@ Coverage Diff @@
## main #221 +/- ##
==========================================
- Coverage 91.88% 91.71% -0.18%
==========================================
Files 8 8
Lines 826 833 +7
==========================================
+ Hits 759 764 +5
- Misses 67 69 +2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, one note about adding a docstring.
|
|
||
| def get_real_child_model(self, data: Dict) -> str: | ||
| def get_real_child_model(self, data: Union[Dict, str]) -> str: | ||
| raise NotImplementedError() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth putting a note in here that this method will be overridden in the auto-generated class if the model has a discriminator.
|
Also confirmed this works against a real service with enums. |
* Add test case with enum property * Add fix for issue * Add docstring as per review comment
Closes #220
This PR adds support for Enums as proeprties of models, it resurrects the code that I excised when we did some refactoring, and makes it slightly clearer what's going on. As part of model deserialization, if the model has no properties then we check if it has an implementation for
get_real_child_model(). If it does not then we assume that it is an enum and return the string value.This is certainly not an ideal approach, we should look to make a further improvement to this in the future, but that will likely require a major release of downstream packages to use an updated client generator. For the moment this is a patch level fix that gets enums working properly.
One test in each direction was added, and they both pass with the changes.